Merge "Use time forcing methods to avoid WANObjectCacheTest flakeiness"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 28 Nov 2017 20:58:08 +0000 (20:58 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 28 Nov 2017 20:58:08 +0000 (20:58 +0000)
autoload.php
includes/DefaultSettings.php
includes/jobqueue/jobs/ClearUserWatchlistJob.php [new file with mode: 0644]
includes/libs/filebackend/SwiftFileBackend.php
includes/watcheditem/WatchedItemStore.php
maintenance/refreshFileHeaders.php
resources/src/mediawiki.special/mediawiki.special.preferences.styles.css
tests/phpunit/includes/filebackend/SwiftFileBackendTest.php
tests/phpunit/includes/jobqueue/jobs/ClearUserWatchlistJobTest.php [new file with mode: 0644]
tests/selenium/.eslintrc.json
tests/selenium/README.md

index 2231a3f..51daced 100644 (file)
@@ -265,6 +265,7 @@ $wgAutoloadLocalClasses = [
        'CleanupRemovedModules' => __DIR__ . '/maintenance/cleanupRemovedModules.php',
        'CleanupSpam' => __DIR__ . '/maintenance/cleanupSpam.php',
        'ClearInterwikiCache' => __DIR__ . '/maintenance/clearInterwikiCache.php',
+       'ClearUserWatchlistJob' => __DIR__ . '/includes/jobqueue/jobs/ClearUserWatchlistJob.php',
        'CliInstaller' => __DIR__ . '/includes/installer/CliInstaller.php',
        'CloneDatabase' => __DIR__ . '/includes/db/CloneDatabase.php',
        'CodeCleanerGlobalsPass' => __DIR__ . '/maintenance/CodeCleanerGlobalsPass.inc',
index 25be60c..dcbcb6e 100644 (file)
@@ -7429,6 +7429,7 @@ $wgJobClasses = [
        'refreshLinksDynamic' => 'RefreshLinksJob',
        'activityUpdateJob' => 'ActivityUpdateJob',
        'categoryMembershipChange' => 'CategoryMembershipChangeJob',
+       'clearUserWatchlist' => 'ClearUserWatchlistJob',
        'cdnPurge' => 'CdnPurgeJob',
        'enqueue' => 'EnqueueJob', // local queue for multi-DC setups
        'null' => 'NullJob'
diff --git a/includes/jobqueue/jobs/ClearUserWatchlistJob.php b/includes/jobqueue/jobs/ClearUserWatchlistJob.php
new file mode 100644 (file)
index 0000000..17c4b66
--- /dev/null
@@ -0,0 +1,119 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+/**
+ * Job to clear a users watchlist in batches.
+ *
+ * @author Addshore
+ *
+ * @ingroup JobQueue
+ * @since 1.31
+ */
+class ClearUserWatchlistJob extends Job {
+
+       /**
+        * @param User $user User to clear the watchlist for.
+        * @param int $maxWatchlistId The maximum wl_id at the time the job was first created.
+        *
+        * @return ClearUserWatchlistJob
+        */
+       public static function newForUser( User $user, $maxWatchlistId ) {
+               return new self(
+                       null,
+                       [ 'userId' => $user->getId(), 'maxWatchlistId' => $maxWatchlistId ]
+               );
+       }
+
+       /**
+        * @param Title|null $title Not used by this job.
+        * @param array $params
+        *  - batchSize,      Number of watchlist entries to remove at once.
+        *  - userId,         The ID for the user whose watchlist is being cleared.
+        *  - maxWatchlistId, The maximum wl_id at the time the job was first created,
+        */
+       public function __construct( Title $title = null, array $params ) {
+               if ( !array_key_exists( 'batchSize', $params ) ) {
+                       $params['batchSize'] = 1000;
+               }
+
+               parent::__construct(
+                       'clearUserWatchlist',
+                       SpecialPage::getTitleFor( 'EditWatchlist', 'clear' ),
+                       $params
+               );
+
+               $this->removeDuplicates = true;
+       }
+
+       public function run() {
+               $userId = $this->params['userId'];
+               $maxWatchlistId = $this->params['maxWatchlistId'];
+
+               $loadBalancer = MediaWikiServices::getInstance()->getDBLoadBalancer();
+               $dbw = $loadBalancer->getConnection( DB_MASTER );
+               $dbr = $loadBalancer->getConnection( DB_REPLICA, [ 'watchlist' ] );
+
+               // Wait before lock to try to reduce time waiting in the lock.
+               if ( !$loadBalancer->safeWaitForMasterPos( $dbr ) ) {
+                       $this->setLastError( 'Timed out while waiting for slave to catch up before lock' );
+                       return false;
+               }
+
+               // Use a named lock so that jobs for this user see each others' changes
+               $lockKey = "ClearUserWatchlistJob:$userId";
+               $scopedLock = $dbw->getScopedLockAndFlush( $lockKey, __METHOD__, 10 );
+               if ( !$scopedLock ) {
+                       $this->setLastError( "Could not acquire lock '$lockKey'" );
+                       return false;
+               }
+
+               if ( !$loadBalancer->safeWaitForMasterPos( $dbr ) ) {
+                       $this->setLastError( 'Timed out while waiting for slave to catch up within lock' );
+                       return false;
+               }
+
+               // Clear any stale REPEATABLE-READ snapshot
+               $dbr->flushSnapshot( __METHOD__ );
+
+               $watchlistIds = $dbr->selectFieldValues(
+                       'watchlist',
+                       'wl_id',
+                       [
+                               'wl_user' => $userId,
+                               'wl_id <= ' . $maxWatchlistId
+                       ],
+                       __METHOD__,
+                       [
+                               'ORDER BY' => 'wl_id ASC',
+                               'LIMIT' => $this->params['batchSize'],
+                       ]
+               );
+
+               if ( count( $watchlistIds ) == 0 ) {
+                       return true;
+               }
+
+               $dbw->delete( 'watchlist', [ 'wl_id' => $watchlistIds ], __METHOD__ );
+
+               // Commit changes and remove lock before inserting next job.
+               $lbf = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+               $lbf->commitMasterChanges( __METHOD__ );
+               unset( $scopedLock );
+
+               if ( count( $watchlistIds ) == $this->params['batchSize'] ) {
+                       JobQueueGroup::singleton()->push( new self( $this->getTitle(), $this->getParams() ) );
+               }
+
+               return true;
+       }
+
+       public function getDeduplicationInfo() {
+               $info = parent::getDeduplicationInfo();
+               // This job never has a namespace or title so we can't use it for deduplication
+               unset( $info['namespace'] );
+               unset( $info['title'] );
+               return $info;
+       }
+
+}
index 373ad93..a3f121e 100644 (file)
@@ -181,6 +181,29 @@ class SwiftFileBackend extends FileBackendStore {
         * @param array $params
         * @return array Sanitized value of 'headers' field in $params
         */
+       protected function sanitizeHdrsStrict( array $params ) {
+               if ( !isset( $params['headers'] ) ) {
+                       return [];
+               }
+               $headers = $this->getCustomHeaders( $params ['headers'] );
+               if ( isset( $headers[ 'content-type' ] ) ) {
+                       unset( $headers[ 'content-type' ] );
+               }
+               return $headers;
+       }
+
+       /**
+        * Sanitize and filter the custom headers from a $params array.
+        * Only allows certain "standard" Content- and X-Content- headers.
+        *
+        * When POSTing data, libcurl adds Content-Type: application/x-www-form-urlencoded
+        * if Content-Type is not set, which overwrites the stored Content-Type header
+        * in Swift - therefore for POSTing data do not strip the Content-Type header (the
+        * previously-stored header that has been already read back from swift is sent)
+        *
+        * @param array $params
+        * @return array Sanitized value of 'headers' field in $params
+        */
        protected function sanitizeHdrs( array $params ) {
                return isset( $params['headers'] )
                        ? $this->getCustomHeaders( $params['headers'] )
@@ -197,7 +220,7 @@ class SwiftFileBackend extends FileBackendStore {
                // Normalize casing, and strip out illegal headers
                foreach ( $rawHeaders as $name => $value ) {
                        $name = strtolower( $name );
-                       if ( preg_match( '/^content-(type|length)$/', $name ) ) {
+                       if ( preg_match( '/^content-length$/', $name ) ) {
                                continue; // blacklisted
                        } elseif ( preg_match( '/^(x-)?content-/', $name ) ) {
                                $headers[$name] = $value; // allowed
@@ -276,7 +299,7 @@ class SwiftFileBackend extends FileBackendStore {
                                'etag' => md5( $params['content'] ),
                                'content-type' => $contentType,
                                'x-object-meta-sha1base36' => $sha1Hash
-                       ] + $this->sanitizeHdrs( $params ),
+                       ] + $this->sanitizeHdrsStrict( $params ),
                        'body' => $params['content']
                ] ];
 
@@ -340,7 +363,7 @@ class SwiftFileBackend extends FileBackendStore {
                                'etag' => md5_file( $params['src'] ),
                                'content-type' => $contentType,
                                'x-object-meta-sha1base36' => $sha1Hash
-                       ] + $this->sanitizeHdrs( $params ),
+                       ] + $this->sanitizeHdrsStrict( $params ),
                        'body' => $handle // resource
                ] ];
 
@@ -391,7 +414,7 @@ class SwiftFileBackend extends FileBackendStore {
                        'headers' => [
                                'x-copy-from' => '/' . rawurlencode( $srcCont ) .
                                        '/' . str_replace( "%2F", "/", rawurlencode( $srcRel ) )
-                       ] + $this->sanitizeHdrs( $params ), // extra headers merged into object
+                       ] + $this->sanitizeHdrsStrict( $params ), // extra headers merged into object
                ] ];
 
                $method = __METHOD__;
@@ -440,7 +463,7 @@ class SwiftFileBackend extends FileBackendStore {
                                'headers' => [
                                        'x-copy-from' => '/' . rawurlencode( $srcCont ) .
                                                '/' . str_replace( "%2F", "/", rawurlencode( $srcRel ) )
-                               ] + $this->sanitizeHdrs( $params ) // extra headers merged into object
+                               ] + $this->sanitizeHdrsStrict( $params ) // extra headers merged into object
                        ]
                ];
                if ( "{$srcCont}/{$srcRel}" !== "{$dstCont}/{$dstRel}" ) {
index 094297c..f29bd47 100644 (file)
@@ -212,6 +212,33 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
                return $this->loadBalancer->getConnectionRef( $dbIndex, [ 'watchlist' ] );
        }
 
+       /**
+        * Queues a job that will clear the users watchlist using the Job Queue.
+        *
+        * @since 1.31
+        *
+        * @param User $user
+        */
+       public function clearUserWatchedItemsUsingJobQueue( User $user ) {
+               $job = ClearUserWatchlistJob::newForUser( $user, $this->getMaxId() );
+               // TODO inject me.
+               JobQueueGroup::singleton()->push( $job );
+       }
+
+       /**
+        * @since 1.31
+        * @return int The maximum current wl_id
+        */
+       public function getMaxId() {
+               $dbr = $this->getConnectionRef( DB_REPLICA );
+               return (int)$dbr->selectField(
+                       'watchlist',
+                       'MAX(wl_id)',
+                       '',
+                       __METHOD__
+               );
+       }
+
        /**
         * @since 1.31
         */
index bd625ba..3bae4b8 100644 (file)
@@ -40,6 +40,12 @@ class RefreshFileHeaders extends Maintenance {
                $this->addOption( 'media_type', 'Media type to filter for', false, true );
                $this->addOption( 'major_mime', 'Major mime type to filter for', false, true );
                $this->addOption( 'minor_mime', 'Minor mime type to filter for', false, true );
+               $this->addOption(
+                       'refreshContentType',
+                       'Set true to refresh file content type from mime data in db',
+                       false,
+                       false
+               );
                $this->setBatchSize( 200 );
        }
 
@@ -100,6 +106,9 @@ class RefreshFileHeaders extends Maintenance {
                        foreach ( $res as $row ) {
                                $file = $repo->newFileFromRow( $row );
                                $headers = $file->getContentHeaders();
+                               if ( $this->getOption( 'refreshContentType', false ) ) {
+                                       $headers['Content-Type'] = $row->img_major_mime . '/' . $row->img_minor_mime;
+                               }
 
                                if ( count( $headers ) ) {
                                        $backendOperations[] = [
index 6745695..0ee4ac2 100644 (file)
        display: inline-block;
        vertical-align: middle;
 }
+
+/* Expand the dropdown and textfield of "Time zone" field to the */
+/* usual maximum width and display them on separate lines. */
+#wpTimeCorrection .oo-ui-dropdownInputWidget,
+#wpTimeCorrection .oo-ui-textInputWidget {
+       display: block;
+       max-width: 50em;
+}
+
+#wpTimeCorrection .oo-ui-textInputWidget {
+       margin-top: 0.5em;
+}
index 9cd2b10..b57af25 100644 (file)
@@ -33,6 +33,68 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                );
        }
 
+       /**
+        * @dataProvider provider_testSanitizeHdrsStrict
+        */
+       public function testSanitizeHdrsStrict( $raw, $sanitized ) {
+               $hdrs = $this->backend->sanitizeHdrsStrict( [ 'headers' => $raw ] );
+
+               $this->assertEquals( $hdrs, $sanitized, 'sanitizeHdrsStrict() has expected result' );
+       }
+
+       public static function provider_testSanitizeHdrsStrict() {
+               return [
+                       [
+                               [
+                                       'content-length' => 345,
+                                       'content-type'   => 'image+bitmap/jpeg',
+                                       'content-disposition' => 'inline',
+                                       'content-duration' => 35.6363,
+                                       'content-Custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ],
+                               [
+                                       'content-disposition' => 'inline',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ]
+                       ],
+                       [
+                               [
+                                       'content-length' => 345,
+                                       'content-type'   => 'image+bitmap/jpeg',
+                                       'content-Disposition' => 'inline; filename=xxx; ' . str_repeat( 'o', 1024 ),
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ],
+                               [
+                                       'content-disposition' => 'inline;filename=xxx',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ]
+                       ],
+                       [
+                               [
+                                       'content-length' => 345,
+                                       'content-type'   => 'image+bitmap/jpeg',
+                                       'content-disposition' => 'filename=' . str_repeat( 'o', 1024 ) . ';inline',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ],
+                               [
+                                       'content-disposition' => '',
+                                       'content-duration' => 35.6363,
+                                       'content-custom' => 'hello',
+                                       'x-content-custom' => 'hello'
+                               ]
+                       ]
+               ];
+       }
+
        /**
         * @dataProvider provider_testSanitizeHdrs
         */
@@ -54,6 +116,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'x-content-custom' => 'hello'
                                ],
                                [
+                                       'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => 'inline',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
@@ -70,6 +133,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'x-content-custom' => 'hello'
                                ],
                                [
+                                       'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => 'inline;filename=xxx',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
@@ -86,6 +150,7 @@ class SwiftFileBackendTest extends MediaWikiTestCase {
                                        'x-content-custom' => 'hello'
                                ],
                                [
+                                       'content-type'   => 'image+bitmap/jpeg',
                                        'content-disposition' => '',
                                        'content-duration' => 35.6363,
                                        'content-custom' => 'hello',
diff --git a/tests/phpunit/includes/jobqueue/jobs/ClearUserWatchlistJobTest.php b/tests/phpunit/includes/jobqueue/jobs/ClearUserWatchlistJobTest.php
new file mode 100644 (file)
index 0000000..385ecb7
--- /dev/null
@@ -0,0 +1,78 @@
+<?php
+use MediaWiki\MediaWikiServices;
+
+/**
+ * @covers ClearUserWatchlistJob
+ *
+ * @group JobQueue
+ * @group Database
+ *
+ * @licence GNU GPL v2+
+ * @author Addshore
+ */
+class ClearUserWatchlistJobTest extends MediaWikiTestCase {
+
+       public function setUp() {
+               parent::setUp();
+               self::$users['ClearUserWatchlistJobTestUser']
+                       = new TestUser( 'ClearUserWatchlistJobTestUser' );
+               $this->runJobs();
+               JobQueueGroup::destroySingletons();
+       }
+
+       private function getUser() {
+               return self::$users['ClearUserWatchlistJobTestUser']->getUser();
+       }
+
+       private function runJobs( $jobLimit = 9999 ) {
+               $runJobs = new RunJobs;
+               $runJobs->loadParamsAndArgs( null, [ 'quiet' => true, 'maxjobs' => $jobLimit ] );
+               $runJobs->execute();
+       }
+
+       private function getWatchedItemStore() {
+               return MediaWikiServices::getInstance()->getWatchedItemStore();
+       }
+
+       public function testRun() {
+               $user = $this->getUser();
+               $watchedItemStore = $this->getWatchedItemStore();
+
+               $watchedItemStore->addWatch( $user, new TitleValue( 0, 'A' ) );
+               $watchedItemStore->addWatch( $user, new TitleValue( 1, 'A' ) );
+               $watchedItemStore->addWatch( $user, new TitleValue( 0, 'B' ) );
+               $watchedItemStore->addWatch( $user, new TitleValue( 1, 'B' ) );
+
+               $maxId = $watchedItemStore->getMaxId();
+
+               $watchedItemStore->addWatch( $user, new TitleValue( 0, 'C' ) );
+               $watchedItemStore->addWatch( $user, new TitleValue( 1, 'C' ) );
+
+               JobQueueGroup::singleton()->push(
+                       new ClearUserWatchlistJob(
+                               null,
+                               [
+                                       'userId' => $user->getId(),
+                                       'batchSize' => 2,
+                                       'maxWatchlistId' => $maxId,
+                               ]
+                       )
+               );
+
+               $this->assertEquals( 1, JobQueueGroup::singleton()->getQueueSizes()['clearUserWatchlist'] );
+               $this->assertEquals( 6, $watchedItemStore->countWatchedItems( $user ) );
+               $this->runJobs( 1 );
+               $this->assertEquals( 1, JobQueueGroup::singleton()->getQueueSizes()['clearUserWatchlist'] );
+               $this->assertEquals( 4, $watchedItemStore->countWatchedItems( $user ) );
+               $this->runJobs( 1 );
+               $this->assertEquals( 1, JobQueueGroup::singleton()->getQueueSizes()['clearUserWatchlist'] );
+               $this->assertEquals( 2, $watchedItemStore->countWatchedItems( $user ) );
+               $this->runJobs( 1 );
+               $this->assertEquals( 0, JobQueueGroup::singleton()->getQueueSizes()['clearUserWatchlist'] );
+               $this->assertEquals( 2, $watchedItemStore->countWatchedItems( $user ) );
+
+               $this->assertTrue( $watchedItemStore->isWatched( $user, new TitleValue( 0, 'C' ) ) );
+               $this->assertTrue( $watchedItemStore->isWatched( $user, new TitleValue( 1, 'C' ) ) );
+       }
+
+}
index db736b7..85fc310 100644 (file)
@@ -5,9 +5,6 @@
                "mocha": true,
                "node": true
        },
-       "parserOptions": {
-               "ecmaVersion": 6
-       },
        "globals": {
                "browser": false
        },
index a14cccb..c895a42 100644 (file)
@@ -30,6 +30,10 @@ Then in another terminal:
     cd tests/selenium
     ../../node_modules/.bin/wdio --spec specs/page.js
 
+To run only one test (name contains string 'preferences'):
+
+    ../../node_modules/.bin/wdio --spec specs/user.js --mochaOpts.grep preferences
+
 The runner reads the config file `wdio.conf.js` and runs the spec listed in
 `page.js`.